[WIP] error handling #246
[WIP] error handling #246
Conversation
emit warning for unknown TCP options instead of throwing error
|
ok thanks |
|
I didn't manage to write tests since for this I would need to create encoded TCP packets with unknown options and I don't have the time to look into how to do that. There were no tests for error handling before, so this PR shouldn't decrease the test coverage. This PR makes error handling more consistent but I'm not sure if it's actually such a good idea not to throw an error if an unknown option or an invalid option length forces the decoder to stop decoding the rest of the options. I guess there are use cases where users actually need to know if an option is set and they might need to know if the reason why it isn't is that the decoder stopped decoding at some point. |
|
Maybe the options object should at least have some property that signals if all options were decoded? |
Hey, I replaced all the logged warnings with emitting an event. Also, unknown TCP options no longer result in a thrown error but also in an emitted warning. That should fix #240 and fix #183. I might add some tests, so please don't merge it yet.
This is a breaking change, so it should be published with the next major version. Before doing so it might be a good idea to agree on a general concept of handling errors. If decoding a higher level protocol fails, should the complete decoding fail or just leave part of it encoded and emit a warning? For example, the DNS decoder does throw some errors. Should lower level protocols handle these?